cc @bitwalker

Could you add a description or link an issue where we can find information about what the goal of the PR is? It is hard to review the changes or judge alternatives without this information.

@PhilippGackstatter @bobbinth This PR makes it possible for us to debug programs executed by the transaction kernel, which is a big improvement in DX. I'd like to try and get this released as part of the v0.22 release cycle/upcoming testnet - just let me know if you need anything on our end to make that go smoothly.

@djolertrk Looks like a couple lint checks failed

Thank you! Not a review yet - but a couple of high-level comments:

• It seems like this PR includes a lot of extraneous changes (e.g., from the AggLayer crate). Could we remove them? (these changes will land soon as a part of feat: migrate miden-agglayer to VM 0.21 and crypto 0.22 #2546)
• It would be great to provide a bit more detail about the goal here. After a brief look, my understanding is that we are trying to control how FastProcessor is instantiated, but I don't yet have full understanding of how this will be used in the compiler. I'm mostly trying to understand if the TransactionProgramFactory pattern is really needed here or if we can do something simpler (e.g., maybe adding a generic type to the TransactionExecutor would suffice).

Also, I wonder if there may be some overlap with #2293 (though, maybe not).

@bobbinth Thanks for the comments! Yes, it does overlap with the #2546 (the AggLayer changes for example), but I needed those so I can test the feature end to end. But yes, if the PR will land soon, I can just rebase on top of it -- so I am doing it.

The immediate consumer here is the debugger, not the compiler. The goal is to let miden-tx::TransactionExecutor reuse its existing transaction preparation / host setup logic, but swap out the program runner used underneath it. The default remains FastProcessor; the debugger plugs in an alternative executor that enables tracing / DAP-driven execution instead of always running straight to completion.

@PhilippGackstatter I've updated PR description as well.

note: Rebased on top of #2546.

note: now rebased on top of next since the #2546 is merged.

@djolertrk I think we've landed on our plan for how to proceed with this. If you can rebase on the latest changes in next, and make the switch to a single trait vs factory + executor traits, I believe this should be basically ready to merge.

@djolertrk I think we've landed on our plan for how to proceed with this. If you can rebase on the latest changes in next, and make the switch to a single trait vs factory + executor traits, I believe this should be basically ready to merge.

Yes. I agree. Let's do it that way (I was already on it -- doing some final testing).
Thanks a lot!

@bobbinth Thank you for the comments!

This can be merged, right? :) @bobbinth @bitwalker

Looks good to me!

I think we can pass the exec options initialized by TransactionExecutor in the places I mentioned, after that, I think we can merge.

@PhilippGackstatter thank you for the feedback!

I guess it is ready now :)

@djolertrk - this is good to merge but the branch needs to be updated (and I can't update it myself for some reason). Could you merge the latest next into this?

@djolertrk - this is good to merge but the branch needs to be updated (and I can't update it myself for some reason). Could you merge the latest next into this?

Done :) Yeah, the reason is that I used fork.

@djolertrk Could you update to next again? Some PRs were merged in the meantime.

@PhilippGackstatter done :)